Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(tests): Unhardcode integration list #3240

Merged

Conversation

rominf
Copy link
Contributor

@rominf rominf commented Jul 3, 2024

Benefits of unhardcoding integration list and disabling auto integrations:

  1. It becomes possible to successfully run tests in environments where certain extra auto integrations get enabled.
  2. There is no need to update hardcoded list when new default integrations are introduced.

The origin of this change is a patch that I had to apply while packaging for Fedora: https://src.fedoraproject.org/rpms/python-sentry-sdk/blob/30f00540cd30328d578dfb84625226dcc7f28969/f/python-sentry-sdk.spec#_52 (https://src.fedoraproject.org/rpms/python-sentry-sdk/blob/30f00540cd30328d578dfb84625226dcc7f28969/f/0002-tests.test_new_scopes_compat_event-compatibility.patch). The patch was necessary because all possible optional dependencies for optional integrations are pre-installed as system packages, so that as many tests as possible could be run. Unfortunately, the patch no longer applies to a new version of the Sentry SDK, so instead of redoing the patch in Fedora, I am submitting this PR.


General Notes

Thank you for contributing to sentry-python!

Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.

For maintainers

Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.

Before running sensitive test suites, please carefully check the PR. Then, apply the Trigger: tests using secrets label. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.

Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.34%. Comparing base (06d5da1) to head (da48ec1).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3240      +/-   ##
==========================================
- Coverage   79.44%   79.34%   -0.10%     
==========================================
  Files         132      132              
  Lines       14278    14278              
  Branches     2999     2999              
==========================================
- Hits        11343    11329      -14     
- Misses       2088     2108      +20     
+ Partials      847      841       -6     

see 5 files with indirect coverage changes

@rominf rominf force-pushed the rominf-tests-integration-list branch from 15ce0f2 to da48ec1 Compare July 11, 2024 16:57
Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, appreciate the contribution!

@szokeasaurusrex szokeasaurusrex added the Trigger: tests using secrets PR code is safe; run CI label Jul 15, 2024
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) July 15, 2024 13:03
auto-merge was automatically disabled July 15, 2024 15:01

Head branch was pushed to by a user without write access

@rominf rominf force-pushed the rominf-tests-integration-list branch from da48ec1 to 33ba99d Compare July 15, 2024 15:01
@github-actions github-actions bot removed the Trigger: tests using secrets PR code is safe; run CI label Jul 15, 2024
@rominf
Copy link
Contributor Author

rominf commented Jul 15, 2024

GitHub told to update the branch.

image

I rebased it on master.

@rominf
Copy link
Contributor Author

rominf commented Jul 16, 2024

@szokeasaurusrex It happened again:
image

What should I do?

@szokeasaurusrex
Copy link
Member

@rominf I can handle getting the PR merged from here

Benefits of unhardcoding integration list and disabling auto
integrations:
1. It becomes possible to successfully run tests in environments where
   certain extra auto integrations get enabled.
2. There is no need to update hardcoded list when new default
   integrations are introduced.
@szokeasaurusrex szokeasaurusrex added the Trigger: tests using secrets PR code is safe; run CI label Jul 16, 2024
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) July 16, 2024 11:03
@szokeasaurusrex szokeasaurusrex merged commit 7a7874d into getsentry:master Jul 16, 2024
123 of 125 checks passed
@rominf rominf deleted the rominf-tests-integration-list branch July 16, 2024 12:18
arjennienhuis pushed a commit to arjennienhuis/sentry-python that referenced this pull request Sep 30, 2024
Benefits of unhardcoding integration list and disabling auto
integrations:
1. It becomes possible to successfully run tests in environments where
   certain extra auto integrations get enabled.
2. There is no need to update hardcoded list when new default
   integrations are introduced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants